feat: LHCb modules migration by AcquaDiGiorgio · Pull Request #97 · DIRACGrid/dirac-cwl

AcquaDiGiorgio · 2026-02-04T15:27:36Z

First approach, needs review.

I tried to use the current Mock classes (MockDataManager), but I don't think this is the correct way. However, it should not vary much.

I open it as a draft because I still need to create 2 more tests and change the interactions with DIRAC.

AcquaDiGiorgio · 2026-02-11T15:36:34Z

There is an issue with the FailoverRequest.
The DIRAC's workflow, during the finalize phase calls sendFailoverRequest, which through the ReqClient sends the stored requests at the workflow_commons dictionary to DIRAC.
Currently, in dirac-cwl we don't have persitency between command executions, so the operations created are lost.

AcquaDiGiorgio · 2026-04-16T13:16:57Z

This PR will now tackle the migration of every single module (starting from the ones used in MCSim) by importing LHCb specific functions instead of rewritting them from the ground up

First approach, needs review

Improve UploadLogFile tests

chore: Create proper wf_commons file update

aldbr · 2026-05-07T12:08:29Z

 """Command classes for workflow pre/post-processing operations."""

+from .analyze_xml_summary import AnalyseXmlSummary
+from .bookkeeping_report import BookeepingReport


Oops, missing a k 🙂

Suggested change

from .bookkeeping_report import BookeepingReport

from .bookkeeping_report import BookkeepingReport

aldbr · 2026-05-07T12:31:12Z

+
+            dsc = DataStoreClient()
+            dsc.addRegister(jobStep)
+            workflow_commons["accounting_registers"] = dsc.__registersList


I doubt this is going to work as registersList is a private attribute.
I guess you would need to add a public getRegisters() method within LHCbDIRAC no?

aldbr · 2026-05-07T12:40:16Z

+            raise
+
+        finally:
+            save_workflow_commons(workflow_commons, workflow_commons_path, failed=failed)


Any reason for not declaring workflow_commons = {} before the try and keeping save_workflow_commons under the if workflow_commons here? You might get exception because workflow_commons won't be defined I think here. This looks correctly handled in upload_log_file for instance

Same issue in analyze_xml_summary and failover_request and upload_output_data

aldbr · 2026-05-07T12:45:23Z

+from dirac_cwl.core.exceptions import WorkflowProcessingException
+
+
+def prepare_lhcb_workflow_commons(workflow_commons_path, extra_mandatory_values=[], extra_default_values={}):


I think it's safer to have immutable default arguments:

Suggested change

def prepare_lhcb_workflow_commons(workflow_commons_path, extra_mandatory_values=[], extra_default_values={}):

def prepare_lhcb_workflow_commons(workflow_commons_path, extra_mandatory_values=None, extra_default_values=None):

Here you might have some surprises if you end up calling prepare_lhcb_workflow_commons multiple times in a row for any reason. (Just in case if you are not aware: https://florimond.dev/en/posts/2018/08/python-mutable-defaults-are-the-source-of-all-evil)

aldbr · 2026-05-07T15:01:02Z

+    finally:
+        os.unlink(wf_backup)
+
+    return True


Looks like the returned bool is never checked. Is that expected?

aldbr · 2026-05-08T14:57:52Z

+
+        except Exception as e:
+            failed = True
+            raise WorkflowProcessingException() from e


No message here?

aldbr · 2026-05-08T14:58:23Z

+            jobStep.setStartTime(now)
+            jobStep.setEndTime(now)
+
+            dataDict = {


This also needs to be typed

aldbr · 2026-05-08T15:00:04Z

+        "mcTCK": None,
+        "condDB_tag": None,
+        "DQ_tag": None,
+        "step_status": S_OK(),


step_status is either Done or Failed, not S_OK/S_ERROR. Check within the StepAccounting module in LHCbDIRAC

Suggested change

"step_status": S_OK(),

"step_status": "Done",

At some point that should just be a boolean but we cannot change it for now (though you can add a comment for later)

aldbr · 2026-05-08T15:02:31Z

+        "inputs",
+        "outputs",  # outputList
+        "executable",
+        "command_id",  # StepID


I am not sure to understand why this is renamed command_id 😅

aldbr · 2026-05-08T15:15:23Z

+from .utils import prepare_lhcb_workflow_commons, save_workflow_commons
+
+
+class FailoverRequest(PostProcessCommand):


It looks like you are not setting the application status, is that expected? (this can be done through diracX)

AcquaDiGiorgio self-assigned this Feb 4, 2026

AcquaDiGiorgio linked an issue Feb 4, 2026 that may be closed by this pull request

LHCb Workflow: UploadLogFile command #87

Closed

4 tasks

AcquaDiGiorgio had a problem deploying to github-pages February 12, 2026 15:59 — with GitHub Actions Failure

AcquaDiGiorgio had a problem deploying to github-pages February 12, 2026 16:02 — with GitHub Actions Failure

AcquaDiGiorgio force-pushed the issue-87-LHCb-UploadLogFile branch from c3f5c70 to 93cb40c Compare February 16, 2026 11:34

aldbr linked an issue Apr 15, 2026 that may be closed by this pull request

Integrate real LHCb workflow commands (Pre/PostExecution) #67

Open

AcquaDiGiorgio changed the title ~~feat: UploadLogFile command implementation~~ feat: LHCb modules migration Apr 16, 2026

AcquaDiGiorgio added 6 commits April 27, 2026 16:16

feat: UploadLogFile command implementation

5075547

First approach, needs review

chore: improve UploadLogFile tests

7a03ef2

feat: Change UploadLogFile DataManager Mocks to real DIRAC Classes

fd12496

Improve UploadLogFile tests

chore: Update project name at imports

8317c9f

chore: setup lhcbdirac dependency to fork

91cef73

feat: Migrate BookkeepingReport command to cwl-dirac

98ccc37

AcquaDiGiorgio force-pushed the issue-87-LHCb-UploadLogFile branch from ca676da to 98ccc37 Compare April 27, 2026 14:20

AcquaDiGiorgio had a problem deploying to github-pages April 27, 2026 14:51 — with GitHub Actions Failure

chore: set lhcbdirac dependency to https instead of ssh

1da58a2

AcquaDiGiorgio force-pushed the issue-87-LHCb-UploadLogFile branch from 6511abe to 1da58a2 Compare April 27, 2026 14:51

AcquaDiGiorgio had a problem deploying to github-pages April 27, 2026 14:53 — with GitHub Actions Failure

chore: remove all DIRAC import mypy type checking

4586f84

AcquaDiGiorgio had a problem deploying to github-pages April 28, 2026 07:28 — with GitHub Actions Failure

AcquaDiGiorgio had a problem deploying to github-pages April 28, 2026 10:04 — with GitHub Actions Failure

AcquaDiGiorgio added 5 commits May 4, 2026 11:17

feat: Migrate FailoverRequest command to cwl-dirac

0ad8e0e

chore(tests): improve command fixtures

bd285c3

feat: Migrate UploadOutputData command to cwl-dirac

26de911

chore: Create proper wf_commons file update

feat: Migrate AnalyseXmlSummary command to cwl-dirac

b87c180

feat: Migrate WorkflowAccounting command to cwl-dirac

32e54b4

AcquaDiGiorgio had a problem deploying to github-pages May 6, 2026 12:20 — with GitHub Actions Failure

feat: Migrate UploadLogFile command to cwl-dirac

f02159a

AcquaDiGiorgio had a problem deploying to github-pages May 6, 2026 14:54 — with GitHub Actions Failure

chore: update pixi.lock

f4d2821

AcquaDiGiorgio had a problem deploying to github-pages May 7, 2026 07:30 — with GitHub Actions Failure

aldbr requested changes May 8, 2026

View reviewed changes

	from .bookkeeping_report import BookeepingReport
	from .bookkeeping_report import BookkeepingReport

		from dirac_cwl.core.exceptions import WorkflowProcessingException


		def prepare_lhcb_workflow_commons(workflow_commons_path, extra_mandatory_values=[], extra_default_values={}):

		from .utils import prepare_lhcb_workflow_commons, save_workflow_commons


		class FailoverRequest(PostProcessCommand):

Conversation

AcquaDiGiorgio commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AcquaDiGiorgio commented Feb 11, 2026

Uh oh!

AcquaDiGiorgio commented Apr 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AcquaDiGiorgio commented Feb 4, 2026 •

edited

Loading